[IA-4175] delete disk wsm dao removal #4752
Conversation
jdcanas
force-pushed
the
jc-remove-wsm-dao-pt2
branch
from
a2c4194
to
9960206
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #4752 +/- ##
============================================
+ Coverage 0 74.17% +74.17%
============================================
Files 0 161 +161
Lines 0 14941 +14941
Branches 0 1226 +1226
============================================
+ Hits 0 11082 +11082
- Misses 0 3859 +3859
... and 157 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
jdcanas
changed the title
| ) | ||
| )(onError) | ||
| } yield res | ||
|
|
||
| override def getWorkspaceStorageContainer(workspaceId: WorkspaceId, authorization: Authorization)(implicit |
There was a problem hiding this comment.
Should this be removed as well? My understanding was that only the Landing Zone related calls would remain in this legacy DAO
There was a problem hiding this comment.
This some pretty specific business logic we do here that dependent on our current decoding I'd rather not redo. I'll comment both.
| import org.broadinstitute.dsde.workbench.leonardo.db.WsmResourceType | ||
| import org.broadinstitute.dsde.workbench.leonardo.util.AppCreationException | ||
| import org.broadinstitute.dsde.workbench.model.TraceId | ||
| import org.broadinstitute.dsde.workbench.openTelemetry.OpenTelemetryMetrics | ||
| import org.http4s._ | ||
| import org.http4s.circe.CirceEntityDecoder._ | ||
| import org.http4s.circe.CirceEntityEncoder._ | ||
| import org.http4s.client.Client | ||
| import org.http4s.client.dsl.Http4sClientDsl | ||
| import org.http4s.headers.{`Content-Type`, Authorization} |
There was a problem hiding this comment.
It would be great to document this file with a Header explaining that this is a legacy DAO that should not be modified other than eventually removing the Landing Zone Logic.
There was a problem hiding this comment.
I am not seeing a commit with these headers / comments. Are these coming as part of another PR? It could be part of this PR honestly
There was a problem hiding this comment.
I added them to the interface WsmDao instead of this concrete implementation
There was a problem hiding this comment.
Sorry could you point me to it? I can't find it 🤔
There was a problem hiding this comment.
Oops apparently I needed to force-push 😅
There was a problem hiding this comment.
Haha no worries, I see it now, looks good 👍
| @@ -76,8 +76,6 @@ final class AdminServiceInterpSpec extends AnyFlatSpec with LeonardoTestSuite wi | |||
| val savedNodepool = makeNodepool(1, cluster1.id).save() | |||
| val app1 = | |||
| makeApp(1, savedNodepool.id, status = AppStatus.Running, appType = AppType.Cromwell, chart = v1Chart).save() | |||
| val app2 = | |||
There was a problem hiding this comment.
Can you also clean up the V2 chart above if this is unused?
| @@ -191,7 +179,6 @@ class AzurePubsubHandlerSpec | |||
| } yield { | |||
| getRuntime.asyncRuntimeFields.flatMap(_.hostIp).isDefined shouldBe true | |||
| getRuntime.status shouldBe RuntimeStatus.Running | |||
| getRuntime.auditInfo.dateAccessed.isAfter(startTime.plusMillis(mockLatencyMillis)) shouldBe true | |||
There was a problem hiding this comment.
@mlilynolting, this line
There was a problem hiding this comment.
iirc this was verifying that the creation time was being recorded as when create finished, instead of when the create request was received. Is the check causing problems?
|
If all the touched actions still work properly i'm 👍 |
jdcanas
changed the title
jdcanas
force-pushed
the
jc-remove-wsm-dao-pt2
branch
from
0b905b7
to
630f026
Compare
Jira ticket: https://broadworkbench.atlassian.net/browse/[ticket_number]
Summary of changes
What
Why
Testing these changes
What to test
Who tested and where
jenkins retestorjenkins multi-test.